Remove the getBoundingClientRect call from SampleGraph#3604
Conversation
Codecov Report
@@ Coverage Diff @@
## main #3604 +/- ##
=======================================
Coverage 88.95% 88.95%
=======================================
Files 257 257
Lines 21352 21352
Branches 5451 5451
=======================================
Hits 18994 18994
Misses 2182 2182
Partials 176 176
Continue to review full report at Codecov.
|
julienw
left a comment
julienw
left a comment
There was a problem hiding this comment.
Thanks, this looks good to me!
From profiles I gathered, this saves some time.
I think the remaining issue isn't about the remaining getBoundingClientRect but that WithSize calls offsetWidth... We need to find a way to compute the widths before setting something in the DOM, instead of interleaving writes and reads...
|
Thanks for the review!
Yeah, we can think about how to do it. |
canova
force-pushed
the
canvas-getboundingclientrect
branch
from
d315721 to
91e523b
Compare
2 participants
Partially fixes #3345
In #3345, @mstange was suggesting that we were doing a lot of layout thrashing. It turned out that we were doing this thrashing because of the
getBoundingClientRectin theSampleGraphcomponent. So I removed this call by wrapping that component with awithSize.There are some snapshot changes, I believe they are changed because we do the layout thrashing differently now and the order got changed. But I don't see anything out of the ordinary in the snapshots.
Here are two profiles with and without this patch, so we can see the difference in the layout thrashing:
Before this PR
After this PR
Note that we still have some
getBoundingClientRectcalls in the "after". But this comes from theOverflowEdgeIndicatorcomponent. I think we should also remove the calls from that component, but that's more complex, because we have twogetBoundingClientRectcalls in there for different divs.